Skip to content

fix(engine): reject Iterator take/drop limits above 2^53 - 1 - #5541

Open
IamYipi wants to merge 1 commit into
boa-dev:mainfrom
IamYipi:fix/iterator-take-drop-limit-range
Open

IamYipi wants to merge 1 commit into
boa-dev:mainfrom
IamYipi:fix/iterator-take-drop-limit-range

Conversation

@IamYipi

@IamYipi IamYipi commented Sep 30, 2026

Copy link
Copy Markdown

This Pull Request implements the limit validation that tc39/ecma262#3776 added to Iterator.prototype.take and Iterator.prototype.drop.

It changes the following:

  • Adds the new step 7 to both methods. When limit converts to a finite Number greater than 2^53 - 1, the method closes the underlying iterator and throws a RangeError. Infinity is still accepted. NaN and negative limits are rejected as before.
  • Renumbers the step comments after the new step.
  • Adds unit tests for limits above Number.MAX_SAFE_INTEGER, equal to it, and Infinity. They also check that a rejected limit closes the underlying iterator.

Test262 results for the full suite, compared with main using boa_tester compare: 6 tests fixed, no regressions.

test/built-ins/Iterator/prototype/take/limit-rangeerror.js
test/built-ins/Iterator/prototype/take/argument-effect-order.js
test/built-ins/Iterator/prototype/take/argument-validation-failure-closes-underlying.js
test/built-ins/Iterator/prototype/drop/limit-rangeerror.js
test/built-ins/Iterator/prototype/drop/argument-effect-order.js
test/built-ins/Iterator/prototype/drop/argument-validation-failure-closes-underlying.js

cargo fmt --all --check is clean. So is cargo clippy -p boa_engine --all-features --all-targets with -D warnings.

ECMA-262 now has Iterator.prototype.take and Iterator.prototype.drop
throw a RangeError, after closing the underlying iterator, when the
limit is finite and greater than 2^53 - 1 (tc39/ecma262#3776). Add that
step to both methods and renumber the step comments that follow it.
@IamYipi
IamYipi requested a review from a team as a code owner September 30, 2026 03:26
@github-actions github-actions Bot added C-Tests Issues and PRs related to the tests. C-Builtins PRs and Issues related to builtins/intrinsics Waiting On Review Waiting on reviews from the maintainers labels Sep 30, 2026
@github-actions github-actions Bot added this to the v0.23 milestone Sep 30, 2026
@github-actions

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,578 53,578 0
Passed 51,439 51,445 +6
Ignored 1,648 1,648 0
Failed 491 485 -6
Panics 0 0 0
Conformance 96.01% 96.02% +0.01%
Fixed tests (6):
test/built-ins/Iterator/prototype/drop/argument-validation-failure-closes-underlying.js (previously Failed)
test/built-ins/Iterator/prototype/drop/limit-rangeerror.js (previously Failed)
test/built-ins/Iterator/prototype/drop/argument-effect-order.js (previously Failed)
test/built-ins/Iterator/prototype/take/argument-validation-failure-closes-underlying.js (previously Failed)
test/built-ins/Iterator/prototype/take/limit-rangeerror.js (previously Failed)
test/built-ins/Iterator/prototype/take/argument-effect-order.js (previously Failed)

Tested main commit: 39cd11214ecb366200157dab96b3919e61e9a391
Tested PR commit: 93f4d41d718658268cbb7cf0e1382111ea78c3af
Compare commits: 39cd112...93f4d41

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant